[rustdoc] Give more information into extracted doctest information #141399
Conversation
rustbot
added
S-waiting-on-review
GuillaumeGomez
changed the title
This comment has been minimized.
This comment has been minimized.
GuillaumeGomez
force-pushed
the
extracted-doctest
branch
from
cd34db0 to
4e95830
Compare
|
Fixed |
src/librustdoc/doctest/make.rs
Outdated
| let mut prog = String::new(); | ||
| let everything_else = self.everything_else.trim(); | ||
| let mut crate_level_code = String::new(); | ||
| let code = self.everything_else.trim().to_string(); |
There was a problem hiding this comment.
Nit: could we call this inner_code to disambiguate?
There was a problem hiding this comment.
Disambiguate with which variable?
There was a problem hiding this comment.
test_code. just looking at those two variable names it's not clear which is the unprocessed input.
There was a problem hiding this comment.
Ah I see. Yeah good point. Gonna rename both.
GuillaumeGomez
force-pushed
the
extracted-doctest
branch
from
4e95830 to
9888eb4
Compare
|
Updated. |
| let (wrapper, line_offset) = | ||
| doctest.generate_unique_doctest(test_code, dont_insert_main, opts, crate_name); | ||
| (code, line_offset) | ||
| (wrapper.to_string(), line_offset) |
There was a problem hiding this comment.
Nit: should s/wrapper/wrapped/, same reasoning as before.
| let (wrapper, full_test_line_offset) = doctest.generate_unique_doctest( | ||
| &scraped_test.text, | ||
| scraped_test.langstr.test_harness, | ||
| &global_opts, | ||
| Some(&global_opts.crate_name), | ||
| ); | ||
| let runnable_test = RunnableDocTest { | ||
| full_test_code, | ||
| full_test_code: wrapper.to_string(), |
There was a problem hiding this comment.
Nit: s/wrapper/wrapped/, same reasoning as before
| @@ -42,7 +43,7 @@ impl ExtractedDocTests { | |||
| .edition(edition) | |||
| .lang_str(&langstr) | |||
| .build(None); | |||
| let (full_test_code, size) = doctest.generate_unique_doctest( | |||
| let (wrapper, _size) = doctest.generate_unique_doctest( | |||
There was a problem hiding this comment.
| let (wrapper, _size) = doctest.generate_unique_doctest( | |
| let (wrapped, _size) = doctest.generate_unique_doctest( |
| @@ -52,21 +53,46 @@ impl ExtractedDocTests { | |||
| file: filename.prefer_remapped_unconditionaly().to_string(), | |||
| line, | |||
| doctest_attributes: langstr.into(), | |||
| doctest_code: if size != 0 { Some(full_test_code) } else { None }, | |||
| doctest_code: match wrapper { | |||
There was a problem hiding this comment.
| doctest_code: match wrapper { | |
| doctest_code: match wrapped { |
| Valid { | ||
| crate_level_code: String, | ||
| wrapper: Option<WrapperInfo>, | ||
| code: String, |
There was a problem hiding this comment.
I'm not sure code is descriptive enough. it doesn't make it clear whether it the original unprocessed code, the fully processed code, or something else. in reality what exactly it contains depends on if we added a main function, so perhaps we could just add a doc comment to the field instead of trying to encapsulate all that complexity in a name.
If we do rename it, we should probably also rename any local variables whose name is derived from this due to the struct pattern sugar, such as in DocTestWrapResult::to_string.
| @@ -210,53 +277,53 @@ impl DocTestBuilder { | |||
| /// lines before the test code begins. | |||
| pub(crate) fn generate_unique_doctest( | |||
| &self, | |||
| test_code: &str, | |||
| doctest_code: &str, | |||
There was a problem hiding this comment.
Nit: in extracted.rs, we use the name doctest_code to refer to the fully processed code, while here we are using it to refer to the unprocessed source. this seems needlessly confusing.
Follow-up of #134531.
This update fragment the doctest code into its sub-parts to give more control to the end users on how they want to use it.
The new JSON looks like this:
{ "format_version":2, "doctests":[ { "file":"$DIR/extract-doctests-result.rs", "line":8, "doctest_attributes":{ "original":"", "should_panic":false, "no_run":false, "ignore":"None", "rust":true, "test_harness":false, "compile_fail":false, "standalone_crate":false, "error_codes":[], "edition":null, "added_css_classes":[], "unknown":[] }, "original_code":"let x = 12;\nOk(())", "doctest_code":{ "crate_level":"#![allow(unused)]\n", "code":"let x = 12;\nOk(())", "wrapper":{ "before":"fn main() { fn _inner() -> core::result::Result<(), impl core::fmt::Debug> {\n", "after":"\n} _inner().unwrap() }", "returns_result":true } }, "name":"$DIR/extract-doctests-result.rs - (line 8)" } ] }for this doctest:
With this, I think it matches what you need @ojeda? If so, once merged I'll update the patch I sent to RfL.
r? @aDotInTheVoid